Skip to content

Speed up npm installation and update oclif - #121

Merged
Yukaii merged 6 commits into
developfrom
feat/120-faster-install
Nov 4, 2026
Merged

Yukaii merged 6 commits into
developfrom
feat/120-faster-install

Conversation

@EastSun5566

Copy link
Copy Markdown
Contributor

Closes #120

Speeds up global npm installation by moving build tools to development dependencies and upgrading oclif. Preserves existing table options, hidden token input, and fish completion.

Validation: build, lint, unit and smoke tests, plus isolated npm installs with and without --ignore-scripts.

@EastSun5566
EastSun5566 marked this pull request as ready for review October 28, 2026 18:32
Copilot AI balanced review requested due to automatic review settings October 28, 2026 18:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved issues break property-key column options, alter null filtering behavior, and prevent package validation on Windows.

3 open issues
What changed in this PR

This PR reduces the CLI’s runtime dependency footprint and upgrades oclif to address slow global npm installation in #120.

Changes:

  • Moves oclif build tooling to development dependencies and removes legacy dependencies.
  • Adds replacement table rendering, hidden token prompting, and fish completion support.
  • Adds compatibility tests and isolated package-install validation, and refreshes documentation.
File Description
tsconfig.json Targets ES2022 output.
test/​table.test.ts Tests table formatting, filtering, sorting, and flags.
test/​prompt-cancel.test.ts Tests cancellation exit status.
test/​helpers/​init.js Removes legacy oclif test initialization.
test/​autocomplete.test.ts Tests fish completion generation and quoting.
src/​table.ts Adds shared table rendering and flags.
src/​commands/​whoami.ts Uses the shared table helper.
src/​commands/​teams.ts Uses the shared table helper.
src/​commands/​team-notes/​index.ts Migrates team-note listing to shared tables.
src/​commands/​team-notes/​create.ts Migrates team-note creation output.
src/​commands/​team-folders/​index.ts Migrates team-folder listing output.
src/​commands/​team-folders/​create.ts Migrates team-folder creation output.
src/​commands/​notes/​index.ts Migrates note listing output.
src/​commands/​notes/​create.ts Migrates note creation output.
src/​commands/​history.ts Migrates browsing-history output.
src/​commands/​folders/​index.ts Migrates folder listing output.
src/​commands/​folders/​create.ts Migrates folder creation output.
src/​commands/​autocomplete/​script.ts Adds fish script sourcing and plugin delegation.
src/​commands/​autocomplete/​index.ts Adds completion setup and cache refresh handling.
src/​command.ts Replaces token prompting and handles cancellation.
src/​autocomplete.ts Generates fish completions and delegates other shells.
scripts/​test-package.mjs Validates isolated installs and packaged CLI behavior.
README.md Documents completion setup and refreshes command help.
package.json Updates dependencies and build/test scripts.
bin/​run Uses the current oclif execution entry point.
bin/​dev Updates development-mode execution.
.mocharc.json Disables native type stripping for tests.
.github/​workflows/​test.yml Adds shell tooling and package-install validation.

Review effort: Balanced

Comment thread scripts/test-package.mjs Resolved
Comment thread src/table.ts Resolved
Comment thread src/table.ts Resolved
Copilot AI balanced review requested due to automatic review settings October 28, 2026 19:16

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The column resolver matches only display headers, so table flags using property names can drop columns, throw filter errors, or silently ignore sorting.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Low severity Add root help and version flags to Fish completion

src/​autocomplete.ts:32

Register the root help and version flags in the generated Fish completion. Every flag entry generated below requires a command path, so hackmd-cli --<TAB> cannot suggest --help or --version, and their short forms are missing too. These are supported entry points configured in package.json and checked by the smoke tests; add root-scoped completion entries for them.

3 resolved since last review

Review effort: Balanced

@EastSun5566
EastSun5566 requested a review from Yukaii October 30, 2026 04:50

@Yukaii Yukaii left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the install footprint work — moving oclif to devDependencies and replacing cli-ux with a local table helper is the right direction for #120, and CI looks green.

Two regressions in src/table.ts vs the old oclif/cli-ux table options; happy to approve once these are fixed:

  1. Column lookup by key — findColumn only matches display headers, so flags that use property names (teamPath, userPath, ownerId, parentFolderId, …) no longer work (--columns, --filter, --sort).
  2. null vs empty string — ?? '' collapses real null fields to "", so filters like Team Path=^null$ stop matching. Prefer defaulting only undefined so null still stringifies as 'null'.

Nits (non-blocking): fish root --help / --version completion, and engines.node is now >=24.

Comment thread src/table.ts Resolved
Comment thread src/table.ts Resolved
Copilot AI balanced review requested due to automatic review settings November 4, 2026 05:14
@EastSun5566
EastSun5566 requested a review from Yukaii November 4, 2026 05:16

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Column lookup breaks selection, sorting, and filtering by property keys; explicit null values become empty strings; and narrow-terminal output can still wrap despite truncation being enabled.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Medium severity Clip output when minimum column widths exceed terminal width

src/​table.ts:108

The sizing loop stops at the header-based minimum widths even when the table still exceeds the terminal width, so default output wraps despite truncation being enabled. For example, the six folder columns require 48 characters before separators, but a 40-column terminal leaves available at 33. Add a fallback that truncates headers or clips rendered lines safely when those minimum widths cannot fit, while preserving --no-truncate.

Review effort: Balanced

@Yukaii
Yukaii merged commit cd31d5f into develop Nov 4, 2026
5 checks passed
@Yukaii
Yukaii deleted the feat/120-faster-install branch November 4, 2026 05:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Speed up npm installation and update oclif dependencies

3 participants